use non-zero exit code on exception #60
Conversation
darshanime-d11
force-pushed
the
exitCode
branch
2 times, most recently
from
0bc8fee to
dd640ea
Compare
|
@dwelch-spike @a-spiker requesting review |
darshanime-d11
force-pushed
the
exitCode
branch
from
dd640ea to
67e1d94
Compare
|
gentle bump please @dwelch-spike @a-spiker |
|
Hi, @darshanime-d11. Thanks for raising this. Please allow us some time, we will be reviewing this PR. |
|
@darshanime-d11 I have reviewed this PR and noticed gaps in addressing the problem statement. The current change only handles exceptions at the main function level but does not account for errors occurring within execution threads or deeper in the stack. It is crucial to ensure that all errors propagate correctly to the main function. Certain functions or threads might encounter errors that are not being surfaced, which could lead to scenarios where failures are not detected and the process does not exit with a non-zero exit code when necessary. Could you provide more context on the use case that led to this change request? Understanding the intent behind it will help assess the best approach for handling errors comprehensively. |
|
@a-spiker We are using the aerospike-loader tool inside one of our microservices to load data into Aerospike clusters. Our API (which inserts data) relies on the exit code of aerospike-loader to determine whether the insert was successful. |
| public static void main(String[] args) throws IOException { | ||
| int exitCode = -1; | ||
| try { | ||
| CommandLine cl = parseArgs(args); |
There was a problem hiding this comment.
Move the parseArgs to launch and let launch take full responsibility for the functionality. In that way, we don't need to include system-lambda for testing.
int exitCode = AerospikeLoad.launch(new String[]{"-h", host, "-p", port, "-n", ns, "-ec", error_count, "-wa", write_action, "-c", "/non/existing/config.json", dataFile});
assertEquals(-1, exitCode);
| CommandLine cl = parseArgs(args); | ||
| exitCode = launch(cl); | ||
| } catch (Exception e) { | ||
| if (log.isDebugEnabled()) { |
There was a problem hiding this comment.
Let's ensure we print the exception error as before.
|
|
||
| public static void main(String[] args) throws IOException { | ||
| long processStart = System.currentTimeMillis(); | ||
| public static void main(String[] args) throws IOException { |
There was a problem hiding this comment.
There seems to be formatting issue with this PR. Please double check.
a-spiker
left a comment
a-spiker
left a comment
There was a problem hiding this comment.
While this PR takes a step towards handling exit codes, it does not yet cover all possible error scenarios comprehensively.
Further contributions would be beneficial to enhance exception handling at a deeper level.
3 participants
currently, aerospike-loader exits using
0exit code, even in the case of an exception. this PR uses a non-zero code for the error flow.